Skip to content

got rid of duplicated file/directory existence implementations / improved errorhandling and testing of FileLister#5350

Merged
danmar merged 7 commits intocppcheck-opensource:mainfrom
firewave:path-x
Aug 23, 2023
Merged

got rid of duplicated file/directory existence implementations / improved errorhandling and testing of FileLister#5350
danmar merged 7 commits intocppcheck-opensource:mainfrom
firewave:path-x

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@firewave firewave marked this pull request as draft August 19, 2023 19:14
@firewave firewave marked this pull request as ready for review August 19, 2023 20:34
@firewave firewave marked this pull request as draft August 19, 2023 20:47
@firewave firewave force-pushed the path-x branch 3 times, most recently from 9b3d599 to 9e49c53 Compare August 20, 2023 19:40
@firewave firewave changed the title got rid of duplicated file/directory existence implementations got rid of duplicated file/directory existence implementations / improved testing of FileLister Aug 22, 2023
@firewave firewave marked this pull request as ready for review August 22, 2023 21:19
@firewave firewave changed the title got rid of duplicated file/directory existence implementations / improved testing of FileLister got rid of duplicated file/directory existence implementations / improved errorhandling and testing of FileLister Aug 23, 2023
@danmar danmar merged commit 499f566 into cppcheck-opensource:main Aug 23, 2023
@firewave firewave deleted the path-x branch August 23, 2023 12:42
@grawlinson
Copy link
Copy Markdown

Hi, this particular commit causes an issue when packaging 2.12.0 for Arch Linux.

It seems to hang indefinitely on TestFileLister, testsuite run shown below (had to Ctrl+C to exit process):

Test64BitPortability:.......
TestAnalyzerInformation:.
TestAssert:.....
TestAstUtils:..........
TestAutoVariables:...................................................................................................
TestBool:.................................
TestBoost:.
TestBufferOverrun:.............................................................................................................................................................................................
TestCharVar:...
TestClangImport:...............................................................................................
TestClass:.......................................................................................................................................................................................
TestCmdlineParser:.............................................................................................................................................................................................
TestColor:.
TestCondition:...................................................................
TestConstructors:........................................................................................................................................................
TestCppcheck:.
TestErrorLogger:.....................
TestExceptionSafety:.....................
TestFileLister:.

It could be because we build packages using systemd-nspawn, I thought it may be useful for cppcheck developers to know this.

@firewave
Copy link
Copy Markdown
Collaborator Author

@grawlinson Thanks for pointing this out. I actually encountered this locally in one of my installations but didn't get around to check and then I injured myself and forgot about it. I will look into this over the weekend.

Also please run testrunner with -q to verify the actual test which is causing this.

As a workaround you can simply disable the test. It was not executed in all cases before since it required a certain root directoy. I added a traversal hack for that which obviously ended up in an endless loop.

@firewave
Copy link
Copy Markdown
Collaborator Author

I am not able to reproduce this (I actually only encountered this once).

The issue is with TestFileLister::recursiveAddFiles() within the calling of findBaseDir() which is walking up the current directory until it hits our root folder. It does this by checking for the existence of the .github folder. So this issue should only occur if the build folder is outside of the source folder or the .github folder was removed. I guess the first case is very likely for packages.

We need to rewrite the test (there's already a TODO about it). I filed https://trac.cppcheck.net/ticket/11993#comment:1 about this.

@grawlinson
Copy link
Copy Markdown

Ah, this will be because of cmake -B build -S source where build directory is deliberately placed outside of the source directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants